feat: add pandas and numpy support #43
Conversation
FilipPyrek
requested review from
Artmann,
andyjakubowski,
jamesbhobbs and
saltenasl
as code owners
📝 WalkthroughWalkthroughThe installer now defines a dependencies array containing deepnote-toolkit[server] (via URL), ipykernel, pandas, and numpy. It logs and installs all dependencies using a single pip install invocation that spreads the array. User-facing messages were updated to reference multiple runtime dependencies. The rest of the flow remains unchanged: create a virtual environment, upgrade pip, then perform kernel-spec installation. Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant VSCode as VS Code Extension
participant Installer as DeepnoteToolkitInstaller
participant Venv as Python venv
participant Pip
User->>VSCode: Trigger toolkit setup
VSCode->>Installer: start()
Installer->>Installer: Create venv (unchanged)
Installer->>Pip: python -m pip install --upgrade pip (unchanged)
Note over Installer,Pip: Install dependencies via single command (new)
Installer->>Pip: pip install [deepnote-toolkit[server]@URL, ipykernel, pandas, numpy]
Pip-->>Installer: Installed packages
Installer->>Installer: Install kernelspec (unchanged)
Installer-->>VSCode: Done
Possibly related PRs
Suggested reviewers
Pre-merge checks✅ Passed checks (3 passed)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #43 +/- ##
=====================================
Coverage 70% 70%
=====================================
Files 512 512
Lines 37506 37506
Branches 4811 4811
=====================================
Hits 26473 26473
Misses 9441 9441
Partials 1592 1592 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (1)
211-254: Verify pandas and numpy installation.Only deepnote-toolkit is verified post-install. pandas/numpy installation could silently fail, breaking dataframe functionality.
Add verification for all dependencies:
// Verify installation -if (await this.isToolkitInstalled(venvInterpreter)) { +if (await this.areAllDependenciesInstalled(venvInterpreter)) { logger.info('deepnote-toolkit installed successfully in venv');Then add a new method:
private async areAllDependenciesInstalled(interpreter: PythonEnvironment): Promise<boolean> { const requiredPackages = ['deepnote_toolkit', 'pandas', 'numpy', 'ipykernel']; try { const processService = await this.processServiceFactory.create(undefined); for (const pkg of requiredPackages) { const result = await processService.exec(interpreter.uri.fsPath, [ '-c', `import ${pkg}; print('installed')` ]); if (!result.stdout.toLowerCase().includes('installed')) { logger.error(`Required package ${pkg} not found`); return false; } } return true; } catch (ex) { logger.debug(`Dependency check failed: ${ex}`); return false; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.node.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
*.node.tsfor Desktop-specific implementations that require full file system access and Python environments
Files:
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Usel10n.t()for user-facing strings
Use typed error classes fromsrc/platform/errors/when throwing or handling errors
Use theILoggerservice instead ofconsole.log
Preserve error details while scrubbing PII in messages and telemetry
Include the Microsoft copyright header in source files
Prefer async/await and handle cancellation withCancellationToken
**/*.{ts,tsx}: Order class members (methods, fields, properties) first by accessibility (public/protected/private) and then alphabetically
Do not add the Microsoft copyright header to new files
Use Uri.joinPath() to construct file paths instead of manual string concatenation
Add a blank line after groups of const declarations and before return statements for readability
Separate third-party imports from local file imports
Files:
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
src/kernels/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)
src/kernels/**/*.ts: Use event-driven updates (EventEmitter) for state changes
Monitor and dispose pending promises to prevent leaks during teardown
Use WeakRef/weak references for notebook/object backreferences to avoid memory leaks
Respect CancellationToken in all async operations
Files:
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: hc2p
PR: deepnote/environments#62
File: gpu/Dockerfile:0-0
Timestamp: 2025-09-25T13:16:13.796Z
Learning: In Deepnote's Docker environment setup, packages installed in the underlying Docker image should be in system site packages, not in virtual environments. Server/kernels are launched in a virtual environment created by the installer, which creates a pth file where system site packages are appended first, giving them higher precedence and making them first in the search path. This is expected behavior for the deepnote-toolkit setup.
🧬 Code graph analysis (1)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (1)
src/kernels/deepnote/types.ts (1)
DEEPNOTE_TOOLKIT_WHEEL_URL(158-158)
🔇 Additional comments (2)
src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (2)
193-193: LGTM!Logging all dependencies clearly.
196-200: LGTM!Spread operator usage is clean and extensible.
| // Define dependencies to install | ||
| const dependencies = [ | ||
| `deepnote-toolkit[server] @ ${DEEPNOTE_TOOLKIT_WHEEL_URL}`, | ||
| 'ipykernel', | ||
| 'pandas', | ||
| 'numpy' | ||
| ]; |
There was a problem hiding this comment.
🧩 Analysis chain
Pin pandas and numpy versions for reproducible builds.
Unversioned dependencies create non-reproducible builds and risk compatibility issues from future major releases.
Apply this diff to pin versions:
const dependencies = [
`deepnote-toolkit[server] @ ${DEEPNOTE_TOOLKIT_WHEEL_URL}`,
'ipykernel',
- 'pandas',
- 'numpy'
+ 'pandas>=2.0.0,<3.0.0',
+ 'numpy>=1.24.0,<2.0.0'
];Verify current production versions:
🏁 Script executed:
#!/bin/bash
# Check latest stable versions of pandas and numpy
pip index versions pandas | head -5
pip index versions numpy | head -5Length of output: 2001
Pin pandas and numpy versions, use l10n.t(), and verify all dependencies
- Replace unversioned
pandas/numpywith'pandas>=2.3.3,<3.0.0'and'numpy>=2.3.3,<3.0.0'in the dependencies array - Wrap the user-facing output in
l10n.t()(currently at line 194) - Extend post-installation checks (
isToolkitInstalled) to verify thatpandasandnumpywere installed
const dependencies = [
`deepnote-toolkit[server] @ ${DEEPNOTE_TOOLKIT_WHEEL_URL}`,
'ipykernel',
- 'pandas',
- 'numpy'
+ 'pandas>=2.3.3,<3.0.0',
+ 'numpy>=2.3.3,<3.0.0'
];🤖 Prompt for AI Agents
In src/kernels/deepnote/deepnoteToolkitInstaller.node.ts around lines 184 to
190, update the dependencies array to use pinned versions for pandas and numpy
('pandas>=2.3.3,<3.0.0' and 'numpy>=2.3.3,<3.0.0') instead of unversioned names;
wrap the user-facing output call at ~line 194 in l10n.t(...) so messages are
localized; and extend the isToolkitInstalled post-installation checks to also
verify that pandas and numpy are present (e.g., by checking importability or pip
list) and return false if either is missing so the installer can retry or
surface an error.
|
|
||
| // Install all dependencies in venv | ||
| logger.info(`Installing dependencies in venv: ${dependencies.join(', ')}`); | ||
| this.outputChannel.appendLine('Installing deepnote-toolkit, ipykernel, and other runtime dependencies...'); |
There was a problem hiding this comment.
Use l10n.t() for user-facing strings and improve clarity.
The message explicitly names two packages but vaguely refers to others. Also, coding guidelines require l10n.t() for user-facing strings.
Apply this diff:
-this.outputChannel.appendLine('Installing deepnote-toolkit, ipykernel, and other runtime dependencies...');
+this.outputChannel.appendLine(l10n.t('Installing runtime dependencies (deepnote-toolkit, ipykernel, pandas, numpy)...'));Note: You'll need to import l10n from vscode at the top of the file.
As per coding guidelines.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/kernels/deepnote/deepnoteToolkitInstaller.node.ts around line 194, the
user-facing message is hard-coded and names two packages while saying "and other
runtime dependencies"; import l10n from 'vscode' at the top of the file and
replace the hard-coded string with a localized message using l10n.t(), e.g. use
l10n.t("Installing runtime dependencies (including deepnote-toolkit and
ipykernel)...") so the message is clearer and localized.
Artmann
left a comment
Artmann
left a comment
There was a problem hiding this comment.
Works great :) The code rabbit suggestions looks good as well :)
|
We found out that toolkit brings both pandas and numpy, so this task was just a false positive alert. |
FilipPyrek
deleted the
fpyrek/grn-4914-include-pandas-and-numpy-in-the-deepnote-kernel
branch
Summary by CodeRabbit